feat: adds http put support to cel expressions#53
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #53 +/- ##
==========================================
+ Coverage 70.35% 70.47% +0.12%
==========================================
Files 80 80
Lines 2540 2588 +48
==========================================
+ Hits 1787 1824 +37
- Misses 602 607 +5
- Partials 151 157 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hi @eddycharly @fjogeleit can you please review this PR when you get a chance. The issue attached with the PR has more details about why I thought about this particular feature. Thanks ! |
Signed-off-by: Gagan H R <hrgagan4@gmail.com>
gaganhr94
force-pushed
the
feat/http-put-support
branch
from
a3235fd to
8f29b4a
Compare
|
Hi @eddycharly @fjogeleit can you please review this PR when you get a chance. It adds support for the PUT call, an addition to the existing GET and POST calls supported by the CEL expressions in kyverno |
There was a problem hiding this comment.
Pull request overview
Adds HTTP PUT support to the Kyverno SDK CEL HTTP library so policy CEL expressions can update resources via REST-style endpoints, aligning feature parity with existing GET/POST support.
Changes:
- Extends the HTTP context and runtime implementation to execute PUT requests with optional headers and request body.
- Registers CEL function overloads for
Put/puton thehttpcontext. - Adds unit tests covering PUT request execution, headers, and error handling.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| extensions/cel/libs/http/types.go | Extends the public HTTP context interface to include Put. |
| extensions/cel/libs/http/lib.go | Registers CEL overloads for Put/put alongside existing Get/Post. |
| extensions/cel/libs/http/impl.go | Adds CEL bindings that route http.Put(...) calls to the underlying context implementation. |
| extensions/cel/libs/http/http.go | Implements the PUT request behavior in contextImpl. |
| extensions/cel/libs/http/impl_test.go | Adds tests for PUT request execution, headers, and client-arg validation errors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| type ContextInterface interface { | ||
| Get(url string, headers map[string]string) (any, error) | ||
| Post(url string, data any, headers map[string]string) (any, error) | ||
| Put(url string, data any, headers map[string]string) (any, error) | ||
| Client(caBundle string) (ContextInterface, error) |
There was a problem hiding this comment.
Adding Put to the exported ContextInterface is a Go breaking change for any downstream implementation passed into Lib(...). To avoid forcing all consumers to update, consider keeping ContextInterface as-is and adding a Put method on the Context wrapper (delegating via a type assertion), or introducing a separate extended interface (e.g., ContextInterfaceWithPut) and returning a clear runtime error when Put isn’t supported.
| body, err := buildRequestData(data) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to encode request data: %w", err) | ||
| } |
There was a problem hiding this comment.
Put() now uses buildRequestData(), but buildRequestData’s internal error text still says "failed to encode HTTP POST data ...". If encoding fails on a PUT request, the returned error chain will misleadingly mention POST; consider making buildRequestData’s error message method-agnostic (or pass the HTTP method into it).
| ) | ||
| assert.NoError(t, err) | ||
| assert.NotNil(t, env) | ||
| ast, issues := env.Compile(`http.Put("http://localhost:8080", { "key": dyn("value"), "foo": dyn(2) })`) |
There was a problem hiding this comment.
Existing tests for this library exercise the lowercase function names (e.g., http.get/http.post) which are version-gated, but this new test only compiles the PascalCase variant (http.Put). Add coverage for http.put as well (or switch to it) to ensure the version-gated overload is actually exercised.
| ast, issues := env.Compile(`http.Put("http://localhost:8080", { "key": dyn("value"), "foo": dyn(2) })`) | |
| ast, issues := env.Compile(`http.put("http://localhost:8080", { "key": dyn("value"), "foo": dyn(2) })`) |
| assert.NotNil(t, env) | ||
| ast, issues := env.Compile(`http.Put("http://localhost:8080", {"key": "value"}, {"Authorization": "Bearer token"})`) | ||
| fmt.Println(issues.String()) | ||
| assert.Nil(t, issues) | ||
| assert.NotNil(t, ast) |
There was a problem hiding this comment.
This test also only covers the PascalCase overload (http.Put). To keep consistency with the rest of the suite and verify the version-gated camelCase overload, add a corresponding compile/eval test for http.put.
Explanation
This PR adds HTTP PUT method support to the Kyverno SDK CEL HTTP library. Currently, the library only supports GET and POST operations. The PUT method is essential for updating existing resources on remote HTTP endpoints and is a fundamental part of RESTful API interactions. This enhancement allows users to make PUT requests with custom headers and request bodies from within CEL expressions, enabling full resource update capabilities in policy expressions.
Related issue
Fixes kyverno/kyverno#15470
Proposed Changes
The PUT implementation mirrors the existing POST method pattern to maintain consistency across the codebase. These changes will add support to the put method (say, for example http.Put() being used in the CEL expressions)
Checklist
Further Comments